NO-ISSUE: add SecurityContext to node image creation and monitor containers by dronenb · Pull Request #2139 · openshift/oc

dronenb · 2025-11-07T20:08:37Z

…pods Signed-off-by: Ben Dronen <dronenb@users.noreply.github.com>

coderabbitai · 2025-11-07T20:08:55Z

Walkthrough

Adds SecurityContext configurations to the node-joiner and node-joiner-monitor containers in the oc adm node-image command's pod specifications. Both containers now enforce non-root execution, disallow privilege escalation, and drop all default capabilities.

Changes

Cohort / File(s)	Change Summary
Security context enforcement `pkg/cli/admin/nodeimage/create.go`, `pkg/cli/admin/nodeimage/monitor.go`	Adds SecurityContext to container specs with `AllowPrivilegeEscalation: false`, `RunAsNonRoot: true`, and `Capabilities.Drop: ["ALL"]`

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Changes are localized to two files with identical security context configurations applied consistently across both containers.
No logic changes, control flow modifications, or new functionality introduced.

Assessment against linked issues

Objective	Addressed	Explanation
Set SecurityContext on node-joiner container in create command [`#2138`]	✅
Set SecurityContext on node-joiner-monitor container in monitor command [`#2138`]	✅

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2025-11-07T20:09:00Z

Hi @dronenb. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

pkg/cli/admin/nodeimage/monitor.go (1)
290-296: LGTM! Security context correctly addresses PodSecurity violations.

The SecurityContext configuration properly satisfies the "restricted:latest" PodSecurity requirements identified in issue #2138. All three mandatory fields are present.

Consider extracting the identical SecurityContext configuration shared with create.go (lines 748-754) into a common helper function to reduce duplication.

Optionally, the boolean pointer pattern &[]bool{false}[0] could be replaced with more idiomatic Go:
SecurityContext: &corev1.SecurityContext{
    AllowPrivilegeEscalation: ptr.To(false),
    RunAsNonRoot:             ptr.To(true),
    Capabilities: &corev1.Capabilities{
        Drop: []corev1.Capability{"ALL"},
    },
},
This requires importing k8s.io/utils/ptr.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1c5f490 and 9318d57.

📒 Files selected for processing (2)

pkg/cli/admin/nodeimage/create.go (1 hunks)
pkg/cli/admin/nodeimage/monitor.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

pkg/cli/admin/nodeimage/create.go
pkg/cli/admin/nodeimage/monitor.go

🔇 Additional comments (1)

pkg/cli/admin/nodeimage/create.go (1)

748-754: LGTM! Security context matches monitor.go implementation.

The SecurityContext correctly addresses the PodSecurity violations for the node-joiner container. See the review comment on monitor.go lines 290-296 for suggested refactors regarding code duplication and the boolean pointer pattern.

zaneb · 2025-12-03T09:45:50Z

/ok-to-test
/lgtm

openshift-ci · 2025-12-03T09:47:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dronenb, zaneb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/cli/admin/nodeimage/OWNERS~~ [zaneb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

zaneb · 2025-12-10T09:55:13Z

/test e2e-agent-compact-ipv4

bfournie · 2026-01-07T15:23:57Z

/test e2e-agent-compact-ipv4

bfournie · 2026-01-09T13:54:52Z

/retitle NO-ISSUE: add SecurityContext to node image creation and monitor containers

openshift-ci-robot · 2026-01-09T13:54:58Z

@dronenb: This pull request explicitly references no jira issue.

Details

In response to this:

Fixes #2138

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

bfournie · 2026-01-09T14:02:00Z

/verified later

openshift-ci-robot · 2026-01-09T14:02:03Z

@bfournie: /verified later <@username> requires at least one GitHub @username to be specified (it can be a comma delimited list). It indicates the engineer(s) that will be performing the verification. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified later

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

bfournie · 2026-01-09T14:02:19Z

/verified later @mhanss

openshift-ci-robot · 2026-01-09T14:02:32Z

@bfournie: This PR has been marked to be verified later by @mhanss.

Details

In response to this:

/verified later @mhanss

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2026-01-09T14:29:45Z

/retest-required

Remaining retests: 0 against base HEAD f532dcb and 2 for PR HEAD 9318d57 in total

openshift-ci-robot · 2026-01-13T12:32:22Z

/retest-required

Remaining retests: 0 against base HEAD ac217ad and 1 for PR HEAD 9318d57 in total

openshift-ci · 2026-01-13T18:10:10Z

@dronenb: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

fix(nodeimage): add PodSecurityContext to image creation and monitor …

9318d57

…pods Signed-off-by: Ben Dronen <dronenb@users.noreply.github.com>

openshift-ci Bot requested review from bfournie and zaneb November 7, 2025 20:08

openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 7, 2025

dronenb changed the title ~~fix(nodeimage): add PodSecurityContext to image creation and monitor pods~~ fix(nodeimage): add SecurityContext to image creation and monitor containers Nov 7, 2025

coderabbitai Bot reviewed Nov 7, 2025

View reviewed changes

openshift-ci Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 3, 2025

openshift-ci Bot assigned zaneb Dec 3, 2025

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2025

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2025

openshift-ci Bot changed the title ~~fix(nodeimage): add SecurityContext to image creation and monitor containers~~ NO-ISSUE: add SecurityContext to node image creation and monitor containers Jan 9, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 9, 2026

openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Jan 9, 2026

openshift-merge-bot Bot merged commit 4df0e94 into openshift:main Jan 14, 2026
16 checks passed

Conversation

dronenb commented Nov 7, 2025

Uh oh!

coderabbitai Bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Assessment against linked issues

Uh oh!

openshift-ci Bot commented Nov 7, 2025

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

zaneb commented Dec 3, 2025

Uh oh!

openshift-ci Bot commented Dec 3, 2025

Uh oh!

zaneb commented Dec 10, 2025

Uh oh!

bfournie commented Jan 7, 2026

Uh oh!

bfournie commented Jan 9, 2026

Uh oh!

openshift-ci-robot commented Jan 9, 2026

Uh oh!

bfournie commented Jan 9, 2026

Uh oh!

openshift-ci-robot commented Jan 9, 2026

Uh oh!

bfournie commented Jan 9, 2026

Uh oh!

openshift-ci-robot commented Jan 9, 2026

Uh oh!

openshift-ci-robot commented Jan 9, 2026

Uh oh!

openshift-ci-robot commented Jan 13, 2026

Uh oh!

openshift-ci Bot commented Jan 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Nov 7, 2025 •

edited

Loading